The rendered link is wrong.

The rendered link is wrong.

Fixed

Thanks for writing this up @Amanieu!

I'm personally would prefer to take this strategy over #1505 as it's more faithful to the atomics we have today (in terms of implementation strategy). There are some small tweaks, however, that I might also change:

• The portability story of the standard library currently is that anything that's platform-specific requires some form of opt-in. This allows code generally using std APIs (or core) to be safely considered as cross platform. You can see this in the std::os::{unix,windows} modules, for example, and I think that we want to take the same approach here as well (platform-specific modules for each sized atomic).

  It's a good point that Atomic{Usize,Isize} are already breaking this convention, however, because not all platforms that core might support have atomic operations. To me the ship has sailed here (as I mentioned earlier), but we can try to be proactive above new types that we add.

  There's also the question of "what does core support". For example is it only tier 1 platforms? Is it all platforms we may eventually want to become tier 1? I would personally say that we should be as flexible as possible (to allow as many possible platforms to become tier 1), which would involve putting these in #[cfg]-specific modules.

• This RFC doesn't explicitly mention the addition of target_has_atomic, but perhaps that could be added?

• For AtomicBool being transmuted to a normal bool the true value is actually all ones, which I think is an invalid bool, so we may not want to say that it can be safely transmuted just yet. I forget why this is the case, unfortunately, that the representation of true is all 1s.

• For target_has_atomic = "ptr", I might recommend just removing that in favor of = "isize" or = "usize"

@alexcrichton

Regarding

(platform-specific modules for each sized atomic).

Are you asking for something like std::atomic_u32::AtomicU32? Or std::target::AtomicU32? Or std::target::mips::AtomicU32? Or something else?

I was persuaded by @Amanieu's comment that there's so much variance within one architecture (e.g. ARM) that we shouldn't have something that is tied to the architecture but rather support via target_has_atomic. Along those lines I would be specifically of something in the std::sync::atomic module, perhaps like std::sync::atomic::target32 or something like that. I'm pretty open to naming, however.

This RFC doesn't explicitly mention the addition of target_has_atomic, but perhaps that could be added?

I believe this is addressed in the "Target support" section.

For AtomicBool being transmuted to a normal bool the true value is actually all ones, which I think is an invalid bool, so we may not want to say that it can be safely transmuted just yet. I forget why this is the case, unfortunately, that the representation of true is all 1s.

The reason for this was to support fetch_nand using the intrinsic. However we can replace it with a compare_exchange loop that performs the operation correctly without resulting in invalid bool values.

For target_has_atomic = "ptr", I might recommend just removing that in favor of = "isize" or = "usize"

I don't really mind, there are plenty of opportunities for bikeshedding here.

I think that we want to take the same approach here as well (platform-specific modules for each sized atomic).

In my view core::sync::atomic is itself already a platform-specific module, and you must check #[cfg(target_has_atomic)] before using it. However std::sync::atomic is guaranteed to have a few types available (all pointer-sized and below) since libstd depends on those types to work. This is similar to libstd depending on floating-point types being available in the language.

There's also the question of "what does core support". For example is it only tier 1 platforms? Is it all platforms we may eventually want to become tier 1? I would personally say that we should be as flexible as possible (to allow as many possible platforms to become tier 1), which would involve putting these in #[cfg]-specific modules.

As I said above, my idea is that libcore supports all platforms, whereas libstd requires a certain minimal feature set to function.

@alexcrichton so std::sync::atomic::target32::AtomicU32 and std::sync::atomic::target16::AtomicU16 and std::sync::atomic::target32::AtomicI32, and so forth, where target32/target64/target128 indicate the number of bits?

@Amanieu

Ah sorry looks like I missed that part of the RFC, my mistake! I wonder though if the fetch_nand intrinsic is more performant than a compare exchange loop? If not then seems fine to change the representation, but it's an indicator that we may not want to quite provide the guarantee that you can freely transmute between these two types.

In my view core::sync::atomic is itself already a platform-specific module, and you must check #[cfg(target_has_atomic)] before using it.

Yeah in some sense this is true, but platforms supporting pointer-sized atomics seems very common where supporting, for example, 64-bit atomics is much less common. I don't think we'd want usage of std::sync::atomic to become a compatibility hazard for now, unfortunately.

As I said above, my idea is that libcore supports all platforms, whereas libstd requires a certain minimal feature set to function.

Ah yes, thanks for pointing this out again! My opinion is that core should be a strict subset of the APIs exposed by std, with no internal differences in hierarchies at all. So I would at least advocate that core should behave the same as std in this respect, which is to have architecture-specific modules.

@jmesmon

Yeah something along those lines (although the names I'd certainly be willing to debate).

Yesterday the libs team was discussing rust-lang/rust#32365 and an interesting point came up. @brson remembered an article in the past which outlined how bitfields can cause problems in C with respect to atomics and word sizes. This is mostly connected to bitfields I think, but it's an interesting data point about smaller-than-word-size atomics using word-size instructions with compare-and-swap to emulate an atomic operation.

My digestion of that article, however, is that it doesn't likely impact Rust for now, as we don't have bitfields. Additionally, if we did have bitfields, it seems like we probably wouldn't want those semantics for other reasons?

@alexcrichton

That gcc bug ( https://gcc.gnu.org/bugzilla/show_bug.cgi?id=52080 ) was that a larger than required read-modify-write was used, which resulted in a non-atomic read-modify-write of a field marked volatile in a struct. It happened to trigger due to some bad handling of bitfields, but the basic concept still exists without them: imagine a machine that can't load/store less than 32bits at a time.

In the article, most of the issue seems to have come from C not having completely specified behavior (ie: the C standard current allows a bit too much flexibility) for volatile qualifiers on struct members.

I suppose our answer to this would mostly hinge on:

• how LLVM thinks about memory (to know if it would try to do an undesired read-modify-write).
• whether we ever need to support a machine which can't read a single byte (for AtomicBool being smaller)
• And whether we simply can't have atomics adjust their size based on platform access requirements (ie: is an atomic's size part of our stability promise).

That gcc bug ( https://gcc.gnu.org/bugzilla/show_bug.cgi?id=52080 ) was that a larger than required read-modify-write was used, which resulted in a non-atomic read-modify-write of a field marked volatile in a struct. It happened to trigger due to some bad handling of bitfields, but the basic concept still exists without them: imagine a machine that can't load less than 32bits at a time.

I think this bug was a case of gcc specifically violating the C++ memory model which forbids a store to one memory location from affecting another memory location. This is explained in this comment which comes from the fix for that gcc bug.

 /* In the C++ memory model, consecutive bit fields in a structure are
    considered one memory location and updating a memory location
    may not store into adjacent memory locations.  */

An architecture that is unable to generate byte-level stores is fundamentally incompatible with the C++ memory model, and would therefore not be supported by Rust since we depend on that memory model.

The only way that an architecture which only has 32-bit stores could be supported in C would be to make char, short, etc all 32 bits wide and not have any types smaller than the smallest native store instruction.

This is mostly connected to bitfields I think, but it's an interesting data point about smaller-than-word-size atomics using word-size instructions with compare-and-swap to emulate an atomic operation.

This isn't actually a problem in this case because the compare-and-swap is atomic, which means that another thread reading data near the emulated atomic will never see an incorrect value and will never have a write which races with the atomic write. This is why this transformation performed by LLVM is valid and does not violate the C++ memory model.

The only problematic situation is when a non-atomic read-modify-write operation is performed because then a store by a second thread could be "lost" if it occurs while another thread is accessing nearby data.

The good news is that all architectures that support the C++ memory model must support byte-level stores, therefore having Rust rely on this feature is perfectly fine.

Here is a rough idea of what the implementation of a generic Atomic<T> type would look like with this RFC:

struct Atomic<T>(UnsafeCell<T>);

impl<T: Copy> Atomic<T> {
    pub fn swap(&self, val: T, order: Ordering) -> T {
        #[cfg(target_has_atomic = "128")]
        if mem::size_of::<T>() == 16 && mem::align_of::<T>() >= 16 {
            let a = &*(self as *const Atomic<T> as *const AtomicU128);
            return mem::transmute(a.swap(mem::transmute(val), order));
        }
        #[cfg(target_has_atomic = "64")]
        if mem::size_of::<T>() == 8 && mem::align_of::<T>() >= 8 {
            let a = &*(self as *const Atomic<T> as *const AtomicU64);
            return mem::transmute(a.swap(mem::transmute(val), order));
        }
        #[cfg(target_has_atomic = "32")]
        if mem::size_of::<T>() == 4 && mem::align_of::<T>() >= 4 {
            let a = &*(self as *const Atomic<T> as *const AtomicU32);
            return mem::transmute(a.swap(mem::transmute(val), order));
        }
        #[cfg(target_has_atomic = "16")]
        if mem::size_of::<T>() == 2 && mem::align_of::<T>() >= 2 {
            let a = &*(self as *const Atomic<T> as *const AtomicU16);
            return mem::transmute(a.swap(mem::transmute(val), order));
        }
        #[cfg(target_has_atomic = "8")]
        if mem::size_of::<T>() == 1 {
            let a = &*(self as *const Atomic<T> as *const AtomicU8);
            return mem::transmute(a.swap(mem::transmute(val), order));
        }

        let result: T = mem::uninitialized();
        atomic_swap_fallback(self.0.get() as *mut (), &val as *const (), &mut result as *mut (), mem::size_of::<T>());
        result
    }
}

I just released a crate which uses this to implement a generic Atomic<T> type with a fallback to spinlocks. It currently only supports pointer-sized types through AtomicUsize on stable rust, but it can easily be extended to support the new atomic types in this RFC.

Nice work @Amanieu! I'm gonna push for this to move into FCP this week hopefully

🔔 This RFC is now entering its week-long final comment period 🔔

I personally am in favor of this RFC as an approach to handling differently sized atomics. In terms of technical details, I've got two pretty minor reservations:

• The AtomicU128 type should probably either be dropped for now or represented as AtomicU64x2. We could later fill this in with a true i128 or u128 if we get those types in the future, but in the meantime if we represent it as (u64, u64) we should likely expose that in the type.
• Right now std::os is the only API-level "compatibility hazard" in the standard library, and I'd prefer to not add std::sync::atomic to that list. Along those lines I think we need to contain these types in some sort of system like std::os to ensure that authors are very aware they are opting in to platform-specific behavior.

I'd personally propose std::arch::{atomic8, atomic16, atomic32, atomic64, atomic128} as modules which contains the relevant AtomicXXX types, but I'm also up for bikeshedding!

• The AtomicU128 type should probably either be dropped for now or represented as AtomicU64x2. We could later fill this in with a true i128 or u128 if we get those types in the future, but in the meantime if we represent it as (u64, u64) we should likely expose that in the type.

The RFC states that 128-bit atomics depend on the i128 RFC, so implementing them can simply be delayed until that RFC is implemented.

I'd personally propose std::arch::{atomic8, atomic16, atomic32, atomic64, atomic128} as modules which contains the relevant AtomicXXX types, but I'm also up for bikeshedding!

This still leaves the issue of what we should do about the existing atomic types on platforms that do not support any form of atomics.

I'd personally propose std::arch::{atomic8, atomic16, atomic32, atomic64, atomic128} as modules

I don't think separate modules necessarily adds value for users of this API (while it may save some extra #[cfg] internally). It just seems to needlessly extend their path. Is there a specific reason you expect users of these values to want the extra wrapping module?

It also isn't clear how things like AtomicUsize would fit in with that pattern (I'm presuming that we'd want all the atomics in one place, and provide type aliases at their historic locations in std to avoid backwards compatibility issues).

Placing atomics in std::arch to indicate they are a compat hazard might make sense though.

@Amanieu

Ah my mistake! Double-word compare and swaps, however, are pretty useful so I hear, so maybe we could get u64x2 today and maybe deprecate it in the future if u128 is stabilized?

This still leaves the issue of what we should do about the existing atomic types on platforms that do not support any form of atomics.

Indeed! I should clarify my thinking there. I'm not really sure what the best course of action is here, but as I've said before I think our hands are basically tied by today's stabilization. I don't think we want to deprecate Atomic{U,I}size and I also don't think we want to follow the same trend as the new types are even more platform specific than what we have today.

We can probably just bolster the documentation to indicate that platforms which don't have AtomicUsize/AtomicIsize will do the compiler-rt fallback (as there's not really much else we could do). We could also perhaps add an atomicptr module to the same location as the other platform-specific modules for consistency.

@jmesmon

I don't think separate modules necessarily adds value for users of this API

It's essentially the same strategy as std::os. You could also say that std::os::unix::process::CommandExt is just lengthening the import of CommandExt, but it's purpose is not to be less ergonomic but serve as a clear indicator that you're opting in to platform-specific behavior. For example all imports of std::os::unix have no hope of compiling on Windows.

Similarly, with these atomics we'd basically just be indicating which platforms have support for these kinds of atomic operations. Only if the target indeed has AtomicU8 operations will the type be available, and this'll be available to code as #[cfg(target_has_atomic = "size")].

For Atomic{U,I}size @Amanieu brought up a good point above that some platforms don't even natively support it, but maybe my above answer helps here as well?

You could also say that std::os::unix::process::CommandExt is just lengthening the import of CommandExt

No, I couldn't. Nothing about CommandExt immediately tells me it's unix or process related (well, there is a hint of process relation, but Command isn't exclusively a process related noun).

With CommandExt the extra path serves to inform the user. With your proposed atomicXX modules, the amount of added information is dubious.

For Atomic{U,I}size @Amanieu brought up a good point above that some platforms don't even natively support it, but maybe my above answer helps here as well?

No, it doesn't entirely clarify that. Are you saying that AtomicUsize would be placed under std::arch::atomicXX::AtomicUsize, where XX varies by the size of usize? That would be actively anti-portable, so I'd presume you're instead proposing something like std::arch::atomic_size::{AtomicUsize,AtomicIsize} or some similar thing with a different name?

Yes, that's what I'd be thinking (atomicptr, atomic_size, etc)

I wanted to cross-post a comment I just wrote elsewhere, related to APIs whose existence is platform/arch dependent.

I just wanted to make a general point that we have a strong need for vision in the area of arch/OS/config/...-dependent APIs.

Examples include:

• Floating point types
• SIMD capabilities
  • Including dynamic detection
• Atomics, as per this RFC
• Android compatiblity issues (with breaking ABI at various releases)

The common thread among all these problems is that we want to expose, in libraries like std, sets of APIs whose availability may depend on various aspects of a platform (OS, architecture, configuration) statically or dynamically. The only place we currently do this is std::os::your_os_here, where each OS's submodule is linked to a cfg. The rationale was that it was easy to audit for non-cross-platform code -- just look for std::os::*.

But this clearly doesn't scale to other kinds of distinctions.

Now, it's not obvious that all of the above examples can be solved by a single mechanism. But I am really eager to see some basic vision for how to approach these questions in Rust, before we go too far down the road of adding specific ad hoc APIs.

If @Amanieu or anyone else is interested in this topic in general, I'd love to work together toward an RFC!

That said, @alexcrichton has been making some concrete suggestions along these lines for the current RFC (via std::arch) -- I'd love to see more elaboration on how we see this module growing over time etc.

@aturon: One thing to consider - module hierarchies are a strict tree, but some functionality may well have fan-in, not just fan-out. As an example, an atomic float type would depend on both 32-bit atomics and float support - and one does not necessarily imply the other. This can be hacked around with pub use/duplication and nesting, but that's decidedly suboptimal IMO.

As the first example that comes to mind, RISC-V has separate extensions for floating-point (F) and atomics (A). Both are in the "General-purpose" set (G = IMAFD - integer core, multiply/divide, atomic, float, double), but there are real cases - cases where Rust is relevant - that may have only one or the other.

In fact, it's very likely that the minion cores on lowRISC will need atomics, and very unlikely they'll need floating-point - and as they'll be largely dedicated to being isolated coprocessors for running drivers, Rust's safety guarantees are very relevant.

Similarly, DSPs are very likely to want floating point, but less likely to need atomics - and Rust's support for zero-cost abstractions would make it much easier to develop for them without sacrificing performance.

As a result, I think it's probably most useful to have a namespace whose members are platform dependent, but not use the names to distinguish platform. They just don't match the non-hierarchical reality.

cc @rust-lang/lang

@aturon

I think it is important to consider how these examples differ from each other:

• Floating-point support: The Rust standard library is heavily dependent on the availability of floating-point types, and I don't see this changing any time soon. This is why the proposal for float-free Rust only involves libcore. The main point is: float-free mode is not supported libstd, it is only available for people who work with just libcore and a small set of hand-picked libraries.
• SIMD support: This is the complete opposite: none of the Rust standard library requires SIMD, so this support can be made completely optional. This is reflected in the SIMD RFC which places everything related to SIMD support in an external crate. The current design of the simd crate has some common SIMD primitives and some extensions in per-architecture modules.
• Atomic support: This is a bit of a hybrid of the two cases above: the Rust standard library requires pointer-sized atomics to work, for things like Arc. However this RFC also proposes supporting platforms which don't have any support for atomic operations, in a libcore-only context just like no-float support. I propose doing this by simply making atomic types in libcore appear/disappear based on platform support. I argue that adding #[cfg(target_has_atomic)] to stable types like AtomicUsize is not a breaking change because it only affects platforms and/or configurations which are not currently supported by Rust. Thus this change does not break any existing code on existing platforms.

(I'm not commenting on the Android ABI issue since I'm not familiar with it)

In conclusion, I don't think there is much of a case for a common mechanism to handle all these cases. I think a namespace like std::arch::* is more suited for holding architecture-specific intrinsic functions than basic functionality like atomic or floating-point types.

@Amanieu Thanks -- I agree that there are substantial differences here. What I'm most interested in having a discussion about is what our overall goals are in terms of platform support/compatibility (which may vary in each of the cases we're discussing), and then the best mechanisms to support those goals. I put some time into this question last week, and have the outlines of a proposal. Unfortunately, I'm on vacation right now and won't be back for a week. I plan to kick of an internals discussion with full details when I get back.

Regardless, though, I don't think these broad considerations should block the RFC from being accepted -- rather, they should block final stabilization, by which point I'd like to have these questions resolved anyway.

I would propose for now that we accept the RFC as-is, that is, with the new atomic types living alongside the existing ones. If and when we have a more complete plan for arch-specific APIs that would demand a different placement, we can move them prior to stabilization. (FWIW, the plan I have in mind would not involve moving these APIs)

@sunfish mentioned to me that it's possible to define lock-free atomics for any size less than a pointer as long as you are willing to spin on the full word. This says to me that maybe it's not necessary to define things like target_has_atomic = "8". IOW if you have target_has_atomic = "ptr" you could automaticaly get everything smaller. Still it is probably nice to be able to conditionally check every size.

@brson regarding spinning on smaller than atomic values, take a look at the discussion started in this comment above: #1543 (comment)

I believe the result is that unless we're able to control data layout in such a way to forbid smaller-than-atomics treated as atomics from occupying the same atomic-unit as non-atomic values, spinning is insufficient.

The libs team discussed this RFC during triage yesterday and the decision was to merge. We decided that the location in std::sync::atomic is good enough for now while we figure out the story for other kinds of platform-specific types elsewhere.

These types will require some extra scrutiny before stabilization, but that doesn't necessarily need to block them landing in the standard library at all, so we felt it was ok to merge and implement for now to let experimentation on nightly with the types.

Thanks again for the RFC @Amanieu and discussion everyone!

Tracking issue: rust-lang/rust#32976

Follow up: I've opened a discuss thread to talk about how we want to handle these APIs in general.